Skip to content

Add ConditionTrait.evaluate() #909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 28, 2025
Merged

Conversation

Uncommon
Copy link
Contributor

Add ConditionTrait.evaluate() so that a condition can be evaluated independent of a Test object.

Motivation:

Currently, the only way a ConditionTrait is evaluated is inside the prepare(for:) method. This makes it difficult and awkward for third-party libraries to utilize these traits because evaluating a condition would require creating a dummy Test to pass to that method.

Modifications:

Add ConditionTrait.evaluate(), and ConditionTrait.Evaluation enum for the return value.

Result:

Public API allows for evaluating a ConditionTrait in any context.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@Uncommon
Copy link
Contributor Author

I'm trying to decide what tests to add. Are existing tests enough, since I pretty much just moved existing logic to a different place?

@grynspan
Copy link
Contributor

We aim for 100% code coverage when testing, so the new function should be tested (even if it is just a trivial "did this return true when I expected it to?" test.)

@grynspan grynspan added enhancement New feature or request public-api Affects public API api-proposal API proposal PRs (documentation only) labels Jan 14, 2025
@grynspan grynspan added this to the Swift 6.x milestone Jan 14, 2025
@Uncommon
Copy link
Contributor Author

I plan to add some tests soon, at which point I think it will be appropriate to take the PR out of draft mode.

@grynspan grynspan requested a review from stmontgomery March 24, 2025 19:12
@Uncommon Uncommon force-pushed the evaluateCondition branch from 2c6c863 to 979f23f Compare March 28, 2025 14:33
Copy link
Contributor

@stmontgomery stmontgomery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Uncommon! Just a couple more comments but this looks close to landing. We can land this before the associated Swift evolution proposal review begins (assuming it moves forward), since the additions are @_spi.

Copy link
Contributor

@stmontgomery stmontgomery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to use "Request Changes" in the comment above.

@stmontgomery
Copy link
Contributor

@swift-ci please test

@stmontgomery stmontgomery merged commit 1d28fa4 into swiftlang:main Mar 28, 2025
3 checks passed
@Uncommon Uncommon deleted the evaluateCondition branch March 28, 2025 18:55
@stmontgomery
Copy link
Contributor

Fixes #903
Fixes rdar://142690932

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-proposal API proposal PRs (documentation only) enhancement New feature or request public-api Affects public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow evaluating ConditionTrait programmatically
3 participants